-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Polyfill for equalTo constraint specializations #28
Conversation
@mergeMarc Thanks for this PR. For my understanding: is there a particular reason why you are using the The assertion methods are already polyfilled in the Note: I'm not adverse to including this, I'd just like to understand the usecase. |
I totally understand. I didn't find other people mentioning this issue anywhere so usage is probably not so wide spread. I need to use it with PHPUnit Mock Objects. To check parameters that are passed to a mocked method call one needs to use a Example:In this excerpt of a simple test case I check if my main application uses my "PersistenceInterface" correctly. I need to include the delta here since the DateTime value of my "Day" will always be off by a few milliseconds. If I run the first version in PHPUnit 9 the test will always fail since the delta is not considered ( PHPUnit < 9 $today = new Day($todayDate, $todaysWeather);
$storage = $this->createMock(PersistenceInterface::class);
$storage->expects($this->once())
->method('updateDay')
->with($this->equalTo($today, 10));
... PHPUnit >= 9 $today = new Day($todayDate, $todaysWeather);
$storage = $this->createMock(PersistenceInterface::class);
$storage->expects($this->once())
->method('updateDay')
->with($this->equalToWithDelta($today, 10));
... |
@mergeMarc Thanks for that additional information. Makes sense to me and I'm happy to include it. There are a few more (minor) changes needed for this PR to be complete. I'd be happy to make those or give you feedback to allow you make them. What would you prefer ? |
@jrfnl Feel free to commit those changes. I would also be available to make them if they require more work. Thank you. |
@mergeMarc No, nothing major. I've just pushed the commit and will give you a moment to look it over. One of us will still need to rebase the PR to make it mergable. |
Ended up adding two more touch up commits. Will stop looking now ;-) (and will squash on merge or rebase). |
The actual methods in PHPUnit do not have any comments so i came up with my own.
The test case already extends assert of course.
Rebased on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mergeMarc Thanks a lot for this PR. Great contribution and well done on the completeness of the PR. This new feature will be included in the next release.
This PR adds the EqualToSpecializations Trait that adds polyfills for the in PHPUnit 9.0.0 introduced methods equalToCanonicalizing, equalToIgnoringCase and equalToWithDelta.
Since PHPUnit Version 9.0.0 equalTo constraints got their own specializations to replicate the assertEquals specializations. (see sebastianbergmann/phpunit@43c01a4) The usage of equalTo with these parameters never got deprecated and also had no replacement until the functionality was removed (see sebastianbergmann/phpunit@e470a3a).
This means that test cases that want to use equalTo with additional configuration cannot be written to work with both PHPUnit 8 (or older) and PHPUnit 9 without the use of a polyfill.